Skip to content

ENH: Validate PET data objects' attributes at instantiation #336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ENH: Validate PET data objects' attributes at instantiation #336

wants to merge 1 commit into from
+352 −20

Conversation

@jhlegarreta
Copy link
Contributor

@jhlegarreta jhlegarreta commented Nov 21, 2025

Validate PET data objects' attributes at instantiation: ensures that the attributes are present and match the expected dimensionalities.

@jhlegarreta
Copy link
Contributor Author

jhlegarreta commented Nov 21, 2025

Depends on PR #335.

@jhlegarreta jhlegarreta force-pushed the enh/validate-pet-data-attrs branch 2 times, most recently from e094509 to a548cbc Compare November 22, 2025 01:08
@jhlegarreta jhlegarreta mentioned this pull request Nov 22, 2025
Open
@jhlegarreta
ENH: Validate PET data objects' attributes at instantiation
96e916b 
Validate PET data objects' attributes at instantiation: ensures that the
attributes are present and match the expected dimensionalities.

Refactor the `from_nii` function so that the required parameters are
present when instantiating the PET instance. Increase consistency with
the `dmri` data module `from_nii` counterpart function.

Refactor the tests accordingly and increase consistency with the `dmri`
data module testing helper functions. Reduces cognitive load and
maintenance burden.

Add additional object instantiation equality checks: check that objects
intantiated through reading NIfTI files equal objects instantiated
directly.
@jhlegarreta jhlegarreta force-pushed the enh/validate-pet-data-attrs branch from a548cbc to 96e916b Compare November 22, 2025 16:47
@jhlegarreta
Copy link
Contributor Author

jhlegarreta commented Nov 22, 2025

@mnoergaard While working on this I've realized that as things stand now on main, there is a risk that two instances of a PET object contain different data if instantiated directly (i.e. PET()) or from a NIfTI file (i.e. from_nii). So, I would like to have your confirm on the following:

  • To build a valid PET instance, the only required piece of information are the data sequence and frame_time, as uptake can be computed from the former, and midframe and total_duration can all be computed from the latter.

I think that answer will allow me to refactor this properly and avoid inconsistencies. Thanks.

@jhlegarreta jhlegarreta linked an issue Nov 22, 2025 that may be closed by this pull request
Ensure the dataset mandatory attributes are present in data instantiation #302
Open
@mnoergaard
Copy link
Contributor

mnoergaard commented Nov 22, 2025

@mnoergaard While working on this I've realized that as things stand now on main, there is a risk that two instances of a PET object contain different data if instantiated directly (i.e. PET()) or from a NIfTI file (i.e. from_nii). So, I would like to have your confirm on the following:

  • To build a valid PET instance, the only required piece of information are the data sequence and frame_time, as uptake can be computed from the former, and midframe and total_duration can all be computed from the latter.

I think that answer will allow me to refactor this properly and avoid inconsistencies. Thanks.

@jhlegarreta - that is correct! Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Ensure the dataset mandatory attributes are present in data instantiation

2 participants

@jhlegarreta @mnoergaard